Skip to content

feat: Enforce relation docID types - #4833

Draft
edjroz wants to merge 8 commits into
sourcenetwork:developfrom
edjroz:feat/enforce-relation-docid-types
Draft

feat: Enforce relation docID types#4833
edjroz wants to merge 8 commits into
sourcenetwork:developfrom
edjroz:feat/enforce-relation-docid-types

Conversation

@edjroz

@edjroz edjroz commented May 25, 2026

Copy link
Copy Markdown
Contributor

Relevant issue(s)

Resolves #4811

Description

Scope

Enforces that a _<relation>ID field must point to a document that (a) exists and has not been soft-deleted in the declared target collection, and (b) is readable by the caller under ACP. Prior to this
change, any arbitrary DocID string — including one from the wrong collection, a deleted document, or a private document — was silently accepted.

The fix is applied in two paths:

  • Local Mutation Validation (Write Path) — AddDocument / UpdateDocument return a hard error if validation fails.
  • P2P Merge Validation (Sync Path) — incoming merged documents are validated after mergeComposites; a missing target is treated as a skip (it may arrive later) rather than an error.

Entry Points

Path Function Called from
Write validateRelationDocIDs collection.add(), collection.update()
Sync validateMergedRelationDocIDs mergeProcessor.executeMerge()
Backup import — skipped via skipRelationValidationContext basicImport

Algorithm — Local Mutation Validation (Write Path)

  1. If the context carries a skip flag (set by backup import), return immediately with no error.
  2. Iterate every field/value pair in the document.
  3. Skip fields that have not been modified in this mutation.
  4. Skip fields that are not a primary FieldKind_DocID relation field.
  5. Skip if the field value is empty (clearing a link is always allowed).
  6. Parse the value as a DocID; skip if malformed.
  7. Derive the companion object-field name and look it up in the schema; skip if not found.
  8. Resolve the target collection via GetRelatedCollection; skip if unknown locally.
  9. Reuse the host collection handle for self-references; otherwise create a lightweight handle for the target collection.
  10. Compute the primary datastore key for the target DocID.
  11. Check whether a document with that key exists and has not been soft-deleted. Return ErrRelationTargetNotFound if not.
  12. Check ACP read permission for the caller on the target document. Return ErrRelationTargetNotFound if denied (same error — avoids leaking that the document exists but is private).
  13. If all dirty relation fields pass, return nil.

Algorithm — P2P Merge Validation (Sync Path)

Outer loop (validateMergedRelationDocIDs):

  1. For each DocID touched during the merge, fetch the document's current state.
  2. If the document is not found or not authorized (e.g. deleted in a race), skip silently.
  3. If any other error occurs, propagate it and abort the merge.
  4. Pass the fetched document to validateMergeRelationDocIDs for per-field validation.

Per-document field walk (validateMergeRelationDocIDs):

  1. Apply the same field-selection logic as the write path to identify primary relation-ID fields and resolve their target collections.
  2. Check whether the target document exists and has not been soft-deleted.
  3. If the target is missing or deleted, skip the field — the document may arrive later via P2P.
  4. If the target exists, the relation is considered valid. No ACP check is performed (no meaningful caller identity in the merge path).
  5. Return nil after all fields are processed.

Out of Scope

  • Cycle detection — self-referential and mutually-referential schemas are not validated for cycles.
  • Wrong-collection detection on the sync path — because DocIDs encode their origin CollectionID in their hash, a DocID from collection A cannot produce a valid DocID for collection B, so this cannot happen from an honest peer. Detecting it for a malicious peer requires scanning all known collections and a pending-marker store — deferred.
  • Blocking merge until target arrives — full deferred validation (store a pending marker, re-validate on target arrival) is out of scope and deferred to a follow-up.

Local Mutation Validation (Write Path)

  • Added validateRelationDocIDs — validates all dirty primary FieldKind_DocID fields on every AddDocument / UpdateDocument call
  • Added docExistsAndNotDeleted — low-level existence check that bypasses ACP to confirm the target document exists regardless of caller permissions
  • Added ACP read-permission check via checkAccessOfDocWithACP — a caller who cannot read the target document cannot link to it
  • Added skipRelationValidationContext — allows backup import to bypass validation for cross-collection forward references
  • Added ErrRelationTargetNotFound error

P2P Merge Validation (Sync Path)

  • Added validateMergedRelationDocIDs on mergeProcessor — runs after mergeComposites, before txn.Commit()
  • Added validateMergeRelationDocIDs — same field-walk as the write path but treats a missing target as a skip rather than an error

Tests

  • Converted 6 previously-passing tests to ExpectedError (relation target not found)
  • Reordered AddDoc sequences in 8 delete/transaction tests to satisfy the new creation-order invariant
  • Fixed TestBackupSelfRefImport_SplitPrimaryRelationWithSecondCollection_NoError to use DocIndex instead of a hardcoded stale DocID
  • Added 7 ACP mutation tests covering no-identity, wrong-identity, owner, granted-read, and public-doc scenarios
  • Added TestP2POneToManyPeerWithAddUpdateRelationValidation_NoError for the sync path happy path
  • Added DocMap support to UpdateDoc action for dynamic DocID substitution in update tests

How has this been tested?

Test changes fall into five categories:

1. Red → ExpectedError — tests that previously passed because the bug allowed dangling relation references; they now assert the new validation error.

  • TestMutationAddOneToOne_WithNonExistentRelation_Error
  • TestMutationAddOneToOne_WithWrongTypeRelation_Error
  • TestMutationAddOneToOne_UseAliasWithNonExistingRelationPrimarySide_Error
  • TestMutationUpdateOneToOne_WithNonExistentRelation_Error
  • TestMutationAddOneToMany_NonExistingRelationManySide_Error
  • TestMutationAddOneToMany_AliasedRelationNameNonExistingRelationManySide_Error

2. Dependency reordering — tests whose AddDoc sequence created a referencing document before its target; reordered to satisfy the new invariant.

  • TestRelationalDeletionOfADocumentUsingSingleKey_Success (and WithAlias, WithMultipleDocumentsWithAlias variants)
  • TestTxnDeletionOfRelatedDoc… (two variants)
  • TestATxnCanReadARecord… (two variants)

3. DocIndex substitution — a test that used a hardcoded Book DocID now uses DocIndex for dynamic resolution.

  • TestBackupSelfRefImport_SplitPrimaryRelationWithSecondCollection_NoError

4. New ACP mutation tests — seven new tests covering the ACP read-permission check on relation fields.

  • TestACP_MutationAdd_RelationTarget_PrivateDoc_NoIdentity_Error
  • TestACP_MutationAdd_RelationTarget_PrivateDoc_WrongIdentity_Error
  • TestACP_MutationAdd_RelationTarget_PrivateDoc_OwnerIdentity_NoError
  • TestACP_MutationAdd_RelationTarget_PrivateDoc_GrantedRead_NoError
  • TestACP_MutationAdd_RelationTarget_PublicDoc_NoIdentity_NoError
  • TestACP_MutationUpdate_RelationTarget_PrivateDoc_NoIdentity_Error
  • TestACP_MutationUpdate_RelationTarget_PrivateDoc_OwnerIdentity_NoError

5. New P2P test — verifies that the sync-path validation does not break normal P2P sync when the relation target is present on the receiving node.

  • TestP2POneToManyPeerWithAddUpdateRelationValidation_NoError

Tested on:

  • MacOS
  • Linux (Docker/golang:1.25.9)

@edjroz
edjroz force-pushed the feat/enforce-relation-docid-types branch from 84492be to 26d5cd5 Compare June 2, 2026 15:39
@fredcarle fredcarle changed the title (feat): Enforce relation docId types feat: Enforce relation docID types Jun 6, 2026
edjroz added 7 commits June 10, 2026 16:10
Adds referential integrity enforcement for relation fields on AddDocument
and UpdateDocument mutations. When a `_<relation>ID` field is set, the
referenced document must exist in the correct target collection; otherwise
a `relation target document not found` error is returned.
Adds post-merge relation validation in executeMerge. After mergeComposites,
each merged document's primary DocID relation fields are checked against the
target collection. A missing target document is treated as a skip (the
referenced doc may not have arrived yet via P2P) so existing sync behaviour
is preserved.
Callers should not be able to link to documents they cannot read.
These tests are currently failing — they document the expected
behaviour before the implementation is in place.

Also adds DocMap support to UpdateDoc so relation fields can reference
dynamically assigned DocIDs without hardcoding strings.
A caller who cannot read a document may not link to it via a relation field. validateRelationDocIDs
now calls checkAccessOfDocWithACP after confirming existence, returning ErrRelationTargetNotFound
on access denial. The shared ACP DAC fixture removes the now-invalid public-employee-to-private-company link.
Covers docExistsAndNotDeleted, validateRelationDocIDs, and
validateMergeRelationDocIDs with real in-memory Badger, no mocks.
…tion

Covers UpdateWithFilter rejecting non-existent and soft-deleted relation
targets, plus transaction isolation scenarios for the relation validator.
Asserts that a merge succeeds when the relation target is absent on the
receiving node — validateMergeRelationDocIDs skips rather than errors.
@edjroz
edjroz force-pushed the feat/enforce-relation-docid-types branch from 26d5cd5 to 9ec1eae Compare June 10, 2026 20:15
…ement

Make setupEmployeeCompanyDB return concrete *collection so tests reach
package-private methods without per-call forcetypeassert. Suppress SA4006
on validateMergeRelationDocIDs's exists check (placeholder for ACP-on-merge
follow-up). Apply gofmt to two test files.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1:1 relationship don't enforce types

1 participant